-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Assertion error Fix - Fixed Assertion true === false error #57
Conversation
This bug has been causing some problems for our team. I went ahead and tested the changes in this PR locally and they seem to be resolved. Would love to see this get properly reviewed/merged by the maintainers. |
Our team also uses an HTTPS app locally, and constantly crashes due to the error this fixes. Please review/merge ASAP! |
+1 It'd be really great to get this fix merged. Many thanks to all people who are involved in this btw! |
+1 |
lib/spdy-transport/priority.js
Outdated
|
||
// Remove the child | ||
this.children.list.splice(index, 1) | ||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not an author or reviewer but I'm stuck with this issue as well. I just noticed this logic could be done a bit easier and without the try catch
:
if (index !== -1 && this.children.list.length >= index) {
this.children.list.splice(index, 1);
}
Would be nice if someone could merge this in. |
+1 |
Anyone merging this soon? |
This needs to be merged -- is anyone maintaining this repo? |
Thank you @deveshmehta7 for the PR. Handling the review, merge and release now |
Published in spdy-transport in 2.1.1. Apologies to everyone on the delay on getting to this one, as described on spdy-http2/node-spdy#339, I'm beyond capacity to be capable to maintain this module, I hope you understand. |
thank you @diasdavid |
@diasdavid can we get this version bumped too https://github.com/spdy-http2/node-spdy/blob/master/package.json#L42? |
@niftylettuce semver should pick on it, right? |
@niftylettuce Remove |
ty
…On Sat, Nov 3, 2018 at 4:10 PM Michał Gołębiowski-Owczarek < ***@***.***> wrote:
@niftylettuce <https://github.com/niftylettuce> Remove node_modules and
package-lock.json and run npm install again and you should get it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#57 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAf7hQ_tD_B18dyWsYbavgsCEiEM9sdCks5urgZXgaJpZM4Wxyph>
.
|
Thank you so much for the fix. This has been annoying me for the past weeks. A small addition to @mgol answer: For people using |
No description provided.